Check for binary file warning and don't print #11
Conversation
|
here's what it looks like: |
|
Hey @heathhenley, Apologies, overlooked your contribution earlier! 🙏🏻 I'll look into the patch but thanks a ton for looking into this problem! P.S. Thanks for the kind words about the project 💛 |
|
I don't have formatting / linting set up locally, probably should for the future anyway so I'll look at getting that set up and fixing the format so that will pass CI. I'm not sure why the opam install should be failing on ci? I only added str in lib/fs/dune 🤷♂️ |
Don't worry about that for now! Once I do #4, it should be trivial to install all required tooling with one command.
The error is mysterious indeed. I haven't touched this project in 5 months, and software is quite vulnerable to bitrot, unfortunately. I'll try to fix CI in a separate PR, so this PR can be rebased afterwards. |
|
@heathhenley FYI, I haven't forgot about your PR! Even though, I got side-tracked by other projects, like FULLY REWRITING MY WEBSITE and also writing an article on category theory. I attempted to fix the CI for this project in: But looks like it still doesn't work, and I feel like I'm blocked by: |
|
Thanks for the note! I think I also bumped into that issue with minttea in a separate project. I was trying to use the latest but couldn't get it set it up properly, I ended up just using the previous version (0.0.2) that's on opam (I was starting from scratch so I didn't matter, just had to follow the 0.0.2 tag's conventions versus the latest). Not sure if you're already using new stuff in this project, but I see >=0.0.2 so maybe pinning it to minttea 0.0.2 and whatever riot that uses solves the build issue here? You probably saw this but I think this is a similar issue going on: leostera/minttea#58 |
|
@heathhenley You're not wrong. I originally bumped into that issue, then I had to figure out pins, and then this new issue. Well, if you depend on the dev unstable version of a project, you'll get unstable results 😅 I know the maintainer is busy I do, however, need to use the latest version as I rely on some patches around configuring FPS. I guess, we'll have to wait! |
chshersh
left a comment
chshersh
left a comment
There was a problem hiding this comment.
Amazing stuff, @heathhenley! Apologies for the delay, I got busy with side quests but now I'm fully back to this project.
I did some work on fixing CI issues in #13. It's merged, so you should be able to sync with the latest version in main and it should work now.
The implementation looks good to me, I left only a few minor comments.
heathhenley
force-pushed
the
hh_check_for_bin_files
branch
from
58de018 to
4fd57d7
Compare
|
Just pushed an update, lmk what you think! |
chshersh
left a comment
chshersh
left a comment
There was a problem hiding this comment.
Looks great! 🏆
I'll merge once CI passes.
2 participants
What do you think about something like this handle the first part of #8 ?
I originally tried to use external utils to get the type ('file --mime` on unix), and that worked nicely but there doesn't seem to be a nice way to do it on windows so I switched to just looking for the bat warning.
I don't know what the ideal way to handle where the regex pattern / warning text, eg whether it's ocaml'y to stick them at the top level of if they should be somewhere else (in the function? factored into some config somewhere, etc).
New to ocaml and was looking at this project for as an example of how to make a tui - thanks for sharing it!